Skip to content

feat(accounts): add platform credential slots - #436

Open
veryCrunchy wants to merge 17 commits into
feature/account-registry-foundationfrom
feature/account-credential-slots
Open

feat(accounts): add platform credential slots#436
veryCrunchy wants to merge 17 commits into
feature/account-registry-foundationfrom
feature/account-credential-slots

Conversation

@veryCrunchy

@veryCrunchy veryCrunchy commented Sep 1, 2026

Copy link
Copy Markdown
Member

Outcome

  • Add bounded Android and desktop credential slots keyed by the opaque local account identity from feat(accounts): add account identity registry foundation #429.
  • Migrate the existing Android encrypted session and desktop secret-store entry without binding one account metadata record to another account password.
  • Keep account selection and removal behind the existing Android sync-scheduling and desktop session-publication guards. Desktop selection is rejected while file ranges, kept-folder hydration, virtual-file providers, or cache moves are active.
  • Add the shared account credential service contract. This is an internal foundation; it does not add the account-picker UI yet.

Advances #172. This PR depends on #429.

Verification

  • Focused Android account credential persistence tests pass
  • Focused desktop credential persistence and secret-reference tests pass
  • Android debug APK assembly passes
  • Desktop distributable creation passes
  • Kotlin architecture checks pass
  • Full repository hygiene passes
  • A new changes/unreleased fragment records the change
  • No credentials, private server data, machine-local paths, or generated output are included

Build-host commands:

./gradlew --no-daemon --max-workers=1 -Pkotlin.incremental=false :androidApp:testDebugUnitTest --tests dev.obiente.nextcloudnative.AndroidPersistedSessionTest :ui:desktopTest --tests dev.obiente.nextcloudnative.app.DesktopAccountCredentialPersistenceTest --tests dev.obiente.nextcloudnative.app.DesktopSecretStoreTest
./gradlew --no-daemon --max-workers=1 -Pkotlin.incremental=false :androidApp:assembleDebug :ui:createDistributable
bash tools/check-kotlin-architecture.sh
bash tools/check-repository.sh

The full source and packaging validation ran on exact #429 parent 78eb576fc8881a246feb1fac8b0b443d4ed1b903. Before publication, #429 advanced to f0696b474a625bd52dfffb29d5695a0a8f996265 with only a one-line automated marketing capture-manifest update. The child source was unchanged. The final child commit only links this PR number in the already-validated changelog fragment.

Compatibility and risk

  • Covered platforms: Android and desktop JVM targets. No device, emulator, live server, macOS keychain, Windows Credential Manager, or Linux Secret Service end-to-end run was performed.
  • Android keeps the existing encrypted SharedPreferences owner and migrates versionless session payloads to a bounded multi-slot store. Corrupt or mismatched metadata and credentials are rejected without overwrite.
  • Desktop moves credentials to opaque account-ID secret references and retains compatibility metadata during migration. Selection does not proceed while old-account file resources are live.
  • Account selection is exposed only through the platform service contract in this slice; user-facing account management remains follow-up work.

Visual changes

Not applicable.

@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #436 / NC Native September 1, 2026 20:42 Destroyed
@obiente-cloud

obiente-cloud Bot commented Sep 1, 2026

Copy link
Copy Markdown

Obiente preview

NC Native · 86bca035817b · Queued

View preview status

View in Obiente

Obiente updates this comment as the preview changes.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-04T06:01:00.327351Z 6f8eb35 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #436 / NC Native September 1, 2026 20:43 Destroyed
@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #436 / NC Native September 1, 2026 20:49 Destroyed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b5e6014e2e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #436 / NC Native September 1, 2026 21:49 Destroyed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d0bb0e52ff

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #436 / NC Native September 3, 2026 20:37 Destroyed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6dabe16e75

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread androidApp/src/main/kotlin/dev/obiente/nextcloudnative/NextcloudFileSyncWorker.kt Outdated
@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #436 / NC Native September 3, 2026 20:51 Destroyed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5849f1719f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #436 / NC Native September 3, 2026 21:06 Destroyed
@veryCrunchy
veryCrunchy force-pushed the feature/account-credential-slots branch from d6d0017 to 4daca37 Compare September 3, 2026 21:08
@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #436 / NC Native September 3, 2026 21:08 Destroyed
@veryCrunchy
veryCrunchy force-pushed the feature/account-credential-slots branch from 4daca37 to f7931ce Compare September 3, 2026 21:14
@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #436 / NC Native September 3, 2026 21:14 Destroyed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f7931ce047

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread androidApp/src/main/kotlin/dev/obiente/nextcloudnative/AndroidPersistedSession.kt Outdated
@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #436 / NC Native September 3, 2026 21:37 Destroyed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0b1e4cb658

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@veryCrunchy
veryCrunchy force-pushed the feature/account-credential-slots branch from 0b1e4cb to c0526df Compare September 4, 2026 00:39
@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #436 / NC Native September 4, 2026 00:39 Destroyed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c0526df17f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@veryCrunchy
veryCrunchy force-pushed the feature/account-credential-slots branch from c0526df to bbc70fc Compare September 4, 2026 01:10
@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #436 / NC Native September 4, 2026 01:10 Destroyed
@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #436 / NC Native September 4, 2026 01:15 Destroyed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bbc70fc564

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@veryCrunchy
veryCrunchy force-pushed the feature/account-credential-slots branch from 28965a7 to 1d310d0 Compare September 4, 2026 01:56
@veryCrunchy
veryCrunchy force-pushed the feature/account-credential-slots branch from 07d146e to ab6601c Compare September 4, 2026 02:56
@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #436 / NC Native September 4, 2026 02:56 Destroyed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ab6601c399

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #436 / NC Native September 4, 2026 03:20 Destroyed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7ce005b917

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@veryCrunchy
veryCrunchy force-pushed the feature/account-credential-slots branch from 7ce005b to b02b997 Compare September 4, 2026 04:02
@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #436 / NC Native September 4, 2026 04:02 Destroyed
@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #436 / NC Native September 4, 2026 04:06 Destroyed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b02b9973ae

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@veryCrunchy
veryCrunchy force-pushed the feature/account-credential-slots branch from 1478f63 to 6f8eb35 Compare September 4, 2026 05:52
@obiente-cloud
obiente-cloud Bot temporarily deployed to Obiente Preview / PR #436 / NC Native September 4, 2026 05:52 Destroyed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6f8eb351d5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

}
}
}
is AndroidAccountCredentialStoreRead.Invalid -> clearInvalidStore(read.encrypted)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Recover independent slots before clearing the session

When the aggregate credential blob is malformed but the credential-free registry and per-account slots remain valid, loadSession() can still authenticate through those slots, yet signing out takes this branch and calls clearInvalidStore, whose empty replacement deletes every account slot rather than only the active account. This silently removes credentials for all retained accounts; fresh evidence beyond the earlier malformed-login issue is that the final clearSession invalid-store branch still bypasses readIndependentCredentialSlotState(). Reconstruct the independent state and remove only its active account before clearing.

AGENTS.md reference: AGENTS.md:L325-L329

Useful? React with 👍 / 👎.

Comment on lines +3763 to +3765
selected = accountOperationGuard.withSyncRunLock {
sessionPublicationGuard.serialize {
accountCredentials.selectAccount(accountId)?.also { session ->

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Serialize desktop pair removal with account selection

When a pair-removal action captured for account A resumes after this selection commits account B, removeFileSyncPair does not acquire accountOperationGuard or withSyncRunLock and does not revalidate the active session. It can therefore call DesktopFileSyncEngine.removePair with A's stale session, perform authenticated cleanup of A's remote upload collections, and delete A's local pair after the switch has completed. The new checks cover sync runs and conflict resolution but not pair removal, so that path should use the same serialization and active-session preflight.

AGENTS.md reference: AGENTS.md:L373-L376

Useful? React with 👍 / 👎.

Comment on lines +499 to +502
removeQueuedUploads = { session ->
incomingShareAccountCleanup.removeForAccount(session)
durableUploadAccountCleanup.removeForAccount(NextcloudDocumentIds.accountKey(session))
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Retire persisted sync pairs when removing an account

When an Android account with configured folder-sync pairs is removed, this cleanup retires incoming-share and multipart-upload jobs but leaves the account's persisted sync pairs intact. If the same account is later added again with the same URL spelling, replaceActiveStateWhileOperationsIdle invokes restorePersistedPairSchedules, which discovers those old pairs and resumes automatic local and remote synchronization without the user recreating them. Beyond the now-cleaned upload stores, this is a separate durable write path from the deleted account lifecycle, so account removal must purge or explicitly retire its sync pairs before completing.

AGENTS.md reference: AGENTS.md:L375-L376

Useful? React with 👍 / 👎.

Comment on lines +555 to +559
val accountLease = acquireAndroidDocumentWritebackAccountLease(
session,
file.path,
services::loadSession,
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Lease every DocumentsProvider mutation across account changes

The new account lease protects only writable-descriptor setup and its delayed writeback, while createDocument, renameDocument, deleteDocument, and moveDocument still capture a session and execute DAV writes without participating in ANDROID_ACCOUNT_OPERATION_GUARD. If selection or removal starts after one of those methods calls requireSession(), the transition can finish before the synchronous mutation runs with the old account credential. Fresh evidence beyond the earlier descriptor finding is that the final lease was added solely in openWritableDocument; acquire and revalidate the same lease around each direct DocumentsProvider mutation as well.

AGENTS.md reference: AGENTS.md:L373-L376

Useful? React with 👍 / 👎.

Comment on lines +1518 to +1520
val accepted = accountOperationGuard.tryActivateResource {
if (!desktopResourceActivationMatchesActiveAccount(activeAccountId(), session.accountId)) {
false

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject stale credentials when activating desktop resources

When a hydration request captured before same-account reauthentication reaches this check after the new password is committed, the account ID is unchanged, so the request is accepted even though its session still contains the superseded credential. The pre-save live-resource check cannot see this not-yet-registered job, and it then performs authenticated reads with the old password and may occupy the hydration slot until failure. Fresh evidence beyond the earlier account-switch race is that final activation validates only activeAccountId() rather than the complete persisted session; compare the supplied session with loadSession() while holding the activation guard.

AGENTS.md reference: AGENTS.md:L373-L376

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant